[#954] Give back the changes a replay which is unwound parked as dependencies by vharseko · Pull Request #985 · OpenIdentityPlatform/OpenDJ

vharseko · 2026-09-09T06:58:53Z

A change which waits for another one is parked - RemotePendingChanges.checkDependencies() puts it in dependentChanges - and stays owned by the replay thread which parked it while that thread goes on to the changes which follow. getNextUpdate() is what hands it out again, to whichever replay thread clears the change it was waiting for, and that thread takes it over: the ownership of a parked change is deliberately held by a thread which is not replaying it right now.

That holds for as long as the parking thread comes back to the pool and keeps taking changes. It does not hold when its replay() is unwound while it still holds one. The change is then owned by a thread which will never come back to it, and nothing else asks for it: putRemoteUpdate() refuses every redelivery of a change a replay thread owns, and a markInProgress() which returns false has the replay thread carry on without reaching getNextUpdate(). It is handed out again only when some other change is replayed on this domain - so on a domain which then goes quiet, that change is where this replica's ServerState, and every change behind it from every master, stops.

The give-back unparks

RemotePendingChanges.releaseParkedChangesOwnedByCurrentThread() drops the owner of every change the calling thread parked and takes it out of dependentChanges in the same step, under both locks. One step for both, because a change which was released while it is still listed as waiting would be handed to the thread getNextUpdate() gives it to and to the thread which takes over the delivery which follows - the double replay the ownership is there to prevent (OPENDJ-1115). That hazard is why the give-back written for #922 left parked changes alone, and unparking them in the same step is what closes it.

The list of what it released is the one allocation of the method past the locks, and it is sized for every change which is waiting before anything is taken out of the set: the rule getNextUpdate() states for itself. This runs on the road out of a JVM which has just refused an allocation, and an allocation which failed once a change had been unparked and released would have moved that change out of the hands which hand it out again, with the caller never told. The locks are taken in the order clear() and getNextUpdate() take them, and the second inside the try of the first: taking a lock another thread holds allocates the node this one waits on, and a throw out of the second would otherwise unwind past the first with that one held by a thread which is ending - every road which lists, commits or gives back a change would wait for it for good.

The changes another thread parked are left alone, as on every other road which reads ownership (#922): that thread may still be inside the dependency checks which parked the change - they park it once per dependency it has - so a change released under it would be listed as waiting again a moment later, and handed out while the delivery which took it over is being replayed.

What is given back stays listed and uncommitted, and stays among the changes the newer ones are checked against, the way a change whose replay failed does: it is not in the data, so it holds the ServerState back and the changes which follow it keep waiting for it.

Where it runs

replay() gives them back at the top of its catch, before the road of the change this thread was replaying: that road restarts the session, and a change which is still owned when the replication server sends it again over the new session is turned down as the duplicate it looks like - the one delivery which could have taken it over. The give-back asks for the restart the way the owned road asks since #981 - sessionRestarts.request(), with the backoff a failing backend is owed, or without it on the two roads the owned road asks without on: a thread which is stopping, and one which an OutOfMemoryError is ending. What is asked for is not what is run - a request is never answered by less than it asked for, so a restart another road asked for with the backoff keeps its wait whichever thread runs it. What is still requested is run afterwards, because that road may have had no restart to run at all: this thread owned no change, or the change it owned was given up on. That run is the latency of the delivery the changes wait for and nothing more - the state checkpointer of #981 runs a request left standing within its tick - and a thread which is stopping leaves the request standing instead, the way abandonReplay() does: the checkpointer runs one restart for every change the threads of the pool hand back on their way out, rather than each of them running one while the configuration change which is stopping them waits.

SessionRestartRequests runs its merge() and its take() once in its constructor, on a thread which can allocate: the first execution of either in a JVM links the call site of the lambda and the VarHandle sites inside AtomicReference, which allocates, and nothing runs them before a replay fails - so on a JVM whose first request is made on the road out of an OutOfMemoryError, the request would have thrown the error again.

Which change this thread owns is read before they are given back, and not after. That read is a plain map lookup which allocates nothing - #958 made it one - and the give-back below it allocates: it builds the list of what it released and the line which reports each one. A throw from it on the road it exists for, a JVM which has just refused an allocation, would otherwise reach the last resort of replay() with nothing read, and the change this thread was replaying would be left listed, uncommitted and owned by a thread which is ending - the wedge #922 is about, one road over. The order of the roads is unchanged: the parked changes are still handed back before the road of the change this thread was replaying restarts the session.

They are handed back without a failure counted against them - they were never applied here, so the give-up budget which decides when this replica skips a change it can not apply is not this delivery's to spend, the way it is not for a change abandoned by a replay thread which is stopping. The delivery which carried one is counted as processed instead: its ack was never published, since the ack of a parked change is published by the delivery which replays it, and that count is of the deliveries this replica took off the session. The receive window it holds is not given back and does not need to be - the session it came over is about to be restarted, and a session which starts is given its window anew.

On a domain whose session has an owner - the domain itself, going away, or a total update into it from the moment it is asked for: sessionHasAnOwner(), the gate abandonReplay(), recoverFromReplayFailure() and restartSession() share - they are released and nothing more, the way abandonReplay() hands a change back on that road: there is no session of this thread's to restart, and the restart it would ask for is refused where it runs, with the request spent on it. The domain forgets its pending changes on its way down, the import forgets them at its end, and a change released for a total update which never begins stays listed until the next failed replay of this domain restarts the session, the way a change recoverFromReplayFailure() releases on that road does - the hold-back #968 left standing and documented in sessionHasAnOwner(), when a request under an owner had nobody to run it; with the state checkpointer of #981 it has, and closing it for the three roads as one policy is #1061. A line per change saying the replication server sends it again would not hold on any of these, and neither would the count. The parked road of replay() runs what is requested under the same gate: a request another thread left standing is not this one's to spend on a restart which is refused.

The last-resort catch of replay() runs the restart outside the guard on the change this thread was replaying, where #958 ran it under that guard: two roads reach it with a request standing and no change of this thread's to hand back, and both are the parked changes' - a give-back which released them, asked for the restart and then threw reporting them, the road an OutOfMemoryError leads to, and a restart the parked road ran and which threw, which asks for one again on its way out. Left to the guard, both would have been left standing for the state checkpointer of #981. It is not run on a domain whose session has an owner, the way no road of a failed replay runs it there: the request stands, and the checkpointer holds it for as long as the total update owns the session, then runs it - or finds it cleared with the pending changes it was made for. The request itself stays where #958 made it, after replayFailed() of the change this thread owned: nothing between the release of a parked change and the request the give-back makes for it can throw - two unlocks, an isEmpty(), two volatile reads and an accumulateAndGet on a constant lambda - so a request made first would cover no road, and would open one: asked for before the release, it could be taken and run by another thread while this one still owned the change, and the delivery the new session brought would be turned down as the duplicate of a change a replay thread owns, with nothing left standing to ask for it again. A give-back which threw before it released anything leaves the parked changes as they were, owned by this thread and handed out by getNextUpdate() to whichever thread clears what they wait for: the last resort can do nothing better for those, and it does not try.

The message this adds is 318: 310-313 are claimed by #935 and 314 by #945; 315, 316 and 317 are in master with #958, 319 and 320 with #945, and 326 and 327 with #959. 318 sits between them and is still nobody else's; 325 is in master with #981.

Which throws reach it

Two roads unwind replay() now, and neither is the one this PR was first written on.

An Error met replaying a change does not reach it: #922 reports it and takes the ordinary road of a failed replay, which gives the change back itself. A throw from publishing the ack does not either, since #958: publishing an ack says nothing about whether the change was applied, so it is reported under ERR_ACK_NOT_PUBLISHED and the replay carries on to the road the change itself decided.

What is left is an OutOfMemoryError, and a throw from what the replay runs once the ack of the delivery is out - the give-back of a change which failed, and the getNextUpdate() which is the one drain of the changes parked behind a change which was committed. The end-to-end test drives the first, on the road of a change which was applied: it is the one road which leaves replay() with no change of this thread's to ask for again - commit() cleared the owner - so the restart the give-back asks for is the only one, and the test is red without it. The second is unwound with the failed change still owned, and the road of that change asks for the same restart.

Tests

RemotePendingChangesTest: a change parked by a replay which is unwound is given back and taken over by the next delivery; a change which was given back is not handed out as a dependency anymore; the changes another thread parked are left alone.
UpdateOperationTest: end to end, with one replay thread so that the change which is parked and the replay which is unwound after it are the same thread's. A change whose replay fails is the barrier - one whose operation is built and then refused (ModifyMsgWhoseOperationRefusesAControl), so that it is asked for again rather than stepped over the way Replication: the permissive-modify check dereferences a null entry DN when the DN does not parse #928 steps over a modify whose entry DN does not parse. The change which follows it on the same entry is published through a broker of its own and parked behind it: a change which travelled the replication server is one only a new session brings back. The replay which is unwound is that of a change which is applied and whose ack runs out of memory (ModifyMsgWhoseAckRunsOutOfMemoryOnceApplied): it leaves replay() with its own change committed and owned by nobody, and the thread ends on the error (Replication: an Error in a replay kills a replay thread the pool never replaces #923). The test watches dependent-changes-size go to zero - the give-back; the thread which is gone was the whole pool, so the delivery the restarted session brings waits in the replay queue and that reading is not a window - then brings the pool back and watches it go to one: the change is parked again behind the barrier, which is what says a new session delivered it. The barrier is then given up on under a budget of 0ms - a change given up on asks for no restart - and the change which was parked is checked to be applied. The barrier is held by a budget which is not spent while the test sets up: it takes the configured replay-give-up-delay route, since Make the replay retry budget of a replication domain configurable instead of a constant with a test-only setter #901 replaced the setters it used. The restart the give-back asks for on that road is owed no backoff, and the case reads getConsecutiveSessionRestarts() - the count [#925] Keep asking for a session restart until it has run #981 exposes for its tests - before the replay is unwound and once the change is back: a restart which waits its backoff out is the one road which moves it, and nothing can move it otherwise in between, since a replay which made it puts the count back to zero only once nothing is failing anymore, and the barrier keeps failing until it is given up. On the way out, the number of replay threads is put back and checked on the normal road, with the finally only calling it, and the barrier is given up after a red as well - handed to the domain, it never travelled the replication server, so nothing but a delivery made here gives it up, and left listed it would keep every later session restart of the class at its backoff and the ServerState behind it; being a cleanup after a failure which is being reported, a throw out of it is logged rather than allowed to replace that failure.
ParkedChangeGiveBackTest: who runs the restart the give-back asks for, on the fixture of ReplayDuringImportTest - the replay runs on the thread of the test, which is also what says whether that thread is stopping, the flag it passes to replay(), and the restart is asked to fail once so that the thread which ran it is the one which met the failure. The thread which gave the parked change back runs it itself: the injected failure comes out with the OutOfMemoryError as a throwable it suppressed, and the restart run again after it brings the session back before replay() returns. A stopping thread leaves it standing: nothing of the failure comes out with the error, the state checkpointer reports it (325) within its tick, and brings the session back after the backoff. The stopping thread is stopped by the ack of the change it applied - the fixture takes the flag replay() reads and sets it in isAssured() before it throws - and not before the replay: stopped before it, the change is abandoned unapplied at the top of its first attempt, the catch finds it still owned, and abandonReplay() asks for a restart of its own next to the give-back's, so the give-back's request would be pinned by nothing. Stopped by the ack, the change is committed and owned by nobody, and the give-back's request is the one which stands - and the case asserts that road, replayed-updates-ok moved by one, since every other assertion of it holds on the abandon road too. That road has a case of its own, a thread stopped before the replay: the change is abandoned unapplied at the top of its first attempt, the ack which says so is what runs out of memory, and the catch hands the change back through its abandon arm - NOTE_REPLAY_ABANDONED_CHANGE for the change, the count unmoved, the parked change given back. Both roads pin that the give-back asks for the restart at once: getConsecutiveSessionRestarts() read before the replay is unwound and once the session is back moves by one, the restart run again after the injected failure, which is the one which waits its backoff out. Pinned on the suppressed throwable rather than on the absence of 325, which races the checkpointer's tick in the instants between the request being made and being taken - the window the case at UpdateOperationTest.java:2430 tolerates.
ReplayDuringImportTest: the changes an unwound replay parked are released and nothing more while a total update owns the session. The request is on its way and the exporter of that test holds it, the barrier's attempts end on an entryUUID search which does not run - the shape of the sibling case - the change which follows it on the same DN is parked by the thread of the test, and that thread's replay of an applied change is then unwound by the same OutOfMemoryError fixture, caught where a replay thread would have ended. What is read: dependent-changes-size to zero - the release - replayed-updates unmoved, no NOTE_REPLAY_PARKED_CHANGE_GIVEN_BACK for the parked change, and the session the answer arrives over still up; the exporter then answers, and the import runs to its end. The count is readable here and not in the end-to-end case because no session restart happens on this road: every road which restarts the session zeroes it under the reading. It is pinned on the import road because that is the one road with an owner a test holds open: disable() forgets the pending changes a moment after it takes the session - the ack which unwinds the replay is published outside the replay lock, so the give-back races remotePendingChanges.clear() - and clears every request and every counter on its way, leaving nothing to read.

Every one of them was watched failing: the unit tests against a give-back which returns nothing and against one which does not check the owner, the end-to-end one against replay() without the call and - on the reading which says a new session delivered the change - against the give-back without its request and the parked road without its run, its backoff reading against a give-back which asks with the backoff on every road, the import case against the give-back without its owner road and against the gate of the previous head, shutdown || disabled, the case of the thread which runs its own restart against the parked road without its run, and the case of the stopping thread against the parked road without its !replayThreadShutdown.get() term and against the give-back without its request - which the case survived while the thread was stopped before the replay - and against itself in that shape, on the reading which says the change was applied; the case of the thread stopped before the replay against the catch without its abandon arm; both cases of the restart's run against a give-back which asks with the backoff on every road.

Run locally on the head before the last round, on master at c6b483127f, one JVM per class: ParkedChangeGiveBackTest 2/2, ReplayDuringImportTest 5/5, UpdateOperationTest 34/34, RemotePendingChangesTest 24/24, SessionRestartBackoffTest 3/3, DependencyTest 3/3; 71 tests, Skipped: 0. The mutants, on this head, each 0/1: the end-to-end case with the request of the give-back and the run of the parked road deleted, the change which was given back must be delivered again by the session which was restarted for it expected [1] but found [0]; with the give-back asking with the backoff on every road, a thread an OutOfMemoryError is ending must not wait the backoff out before it restarts the session for the changes it gave back expected [1] but found [2]; the import case with the owner road of the give-back deleted, and again under shutdown || disabled, a change released while a total update owns the session must not be counted as processed ... expected [1] but found [2], with message 318 in the error log of both; the case of the thread which runs its own restart with the parked road's run deleted, the thread which gave the parked change back must have run the restart it asked for, and met the failure that restart was asked to meet - Expected size: 1 but was: 0; the case of the stopping thread with the !replayThreadShutdown.get() term dropped, a thread which is stopping must not run the restart it asked for - Expecting empty but was: [IllegalStateException: the session of domain o=test could not be started again, as a test asked], and with the give-back's request deleted, the state checkpointer did not run the restart the stopping thread left standing: the failure that restart was asked to meet was never reported - green 1/1 under that mutant with the thread stopped before the replay, since the abandon road's request stood in. On this head, on master at d07bb31897: ParkedChangeGiveBackTest 3/3, ReplayDuringImportTest 6/6, SessionRestartBackoffTest 3/3, UpdateOperationTest 40/40; the case of the stopping thread in its previous shape, red on the reading which says the change was applied, expected [1] but found [0]; the case of the thread stopped before the replay with the abandon arm deleted from the catch, a stopping thread must hand back the change it did not apply - Expecting actual not to be empty; the two cases of the restart's run with the give-back asking with the backoff on every road, expected [2] but found [3].

Built on #958

This is written on top of #958, which is where the give-back on the way out of an unwound replay lives and which gives ownership an owner - without it there is no "the changes this thread parked" to ask for. #958 is in master since 776339a, and this PR is the one commit [#954] on top of it. Rebased across #981 (c6b483127f) since, which replaced the flag the give-back set with SessionRestartRequests: the give-back asks through it now, see above. Rebased onto master at d07bb31897 after that, clean.

Left out

A replay thread which ends while it holds a parked change without anything being thrown: the road a replay abandoned by a stopping thread takes, and - wider - MultimasterReplication.applyConfigurationChange(), which stops the whole pool when ds-cfg-num-update-replay-threads changes and takes the parked changes of every idle thread with it. It is the same defect, but no catch of replay() is on the way and the changes are owned by threads which are gone rather than by the one asking, so it wants a sweep of its own rather than a line here. It is #986.

Fixes #954

vharseko · 2026-09-09T10:14:52Z

Rebased onto the head of #958. That branch grew a third commit while this sat - [#922] Report an ack which could not be published rather than unwind the replay on it - and it takes away the road this PR's end-to-end test was written on, so the rebase was not only textual. replication.properties and RemotePendingChangesTest conflicted where both branches append; LDAPReplicationDomain and UpdateOperationTest merged clean and wrong.

The ordinal is 318

#958 claims 315, 316 and 317 now, which is what its own comment announced. #981 is on the same branch and also took 316, so it lands on 319 rather than on this.

The end-to-end test unwinds the replay another way

It drove ModifyMsgWhoseAckThrows, and an ack which throws is exactly what #958 stopped unwinding replay() on: it is reported under ERR_ACK_NOT_PUBLISHED and the replay carries on to the road the change itself decided. For a change whose replay failed that road is recoverFromReplayFailure() and a return, so replay() is never unwound and the give-back this PR adds is never reached.

It now uses ModifyMsgWhoseReplayIsUnwoundAfterItsAck, from that same commit: the throw is made from getCSN() once the ack is out, past every catch the replay itself has. Watched failing against replay() without the give-back call: a change parked by a replay which was unwound must be given back expected [0] but found [1].

The last resort runs the restart for these too

#958's fallback runs runRequestedSessionRestarts() itself rather than leave a request the two readers of which both need a later failed replay in this domain - but it ran it under owned != null. giveBackParkedChanges() asks for that same restart and can throw formatting its report, which is the road an OutOfMemoryError leads to; on a thread which owns no change of its own that left the changes it had already released unowned with nobody to ask for them. That is the wedge this PR is about, one road over, so the restart is now outside the guard. The request is still made before the changes are reported, so the report is what may be lost, never the restart.

Two javadocs which the third commit made untrue

theChangesParkedAsDependenciesAreNotOwnedByTheThreadWhichParkedThem said a give-back on the way out of an unwound replay "must leave it alone" - true of getChangeOwnedByCurrentThread(), which is what it pins, but not of the give-back as a whole once the parked changes have a road of their own. And this PR's own unit test named "an Error, an exception on the way to the ack" as what unwinds a replay; neither does anymore.

Runs

UpdateOperationTest 26/26, RemotePendingChangesTest 24/24, AssuredReplicationPluginTest 14/14, ModifyConflictTest 36/36, NamingConflictTest 7/7, StateMachineTest 5/5, DependencyTest 3/3 - 115 tests, Skipped: 0.

The description is updated to match.

vharseko · 2026-09-11T04:58:54Z

Rebased onto the head of #958 - the branch conflicted with master and with that PR, which has grown a fourth commit since this was last pushed, [#922] Ask which change to give back without asking the JVM for memory. Nothing here is a new road; what follows is what the rebase had to decide.

The owner is read before the parked changes are given back

That fourth commit turns "which change does this thread own" into a plain map read which takes no lock and allocates nothing, because the give-back on the way out of an unwound replay runs on the road out of a JVM which has just refused an allocation. This PR was written when that lookup still walked the pending changes, and it put its own give-back first.

Left as it merged, the give-back of the parked changes - which allocates: the list of what it released, and the line which reports each one - ran before anything had been read, so a throw from it reached the last resort of replay() with owned still null. The change this thread was replaying would then be left listed, uncommitted and owned by a thread which is ending, and putRemoteUpdate() refuses every later delivery of it: the wedge #922 is about, reached one road over by the fix for #954.

The read is first now, and the give-back follows it. The order of the roads is unchanged - the parked changes are still handed back before the road of the change this thread was replaying restarts the session, which is the whole point of where the call sits.

Two comments the fourth commit made untrue

addDependency() said the give-back on the way out of an unwound replay leaves a parked change alone. True of the change this thread is replaying, and it is what that comment is next to - but not of the give-back as a whole anymore, so it now names the road which does hand a parked change back. The javadoc of the new index said the parked ones "are not this one's to give back" for the same reason; it says which road gives them back instead.

The end-to-end test takes the configured budget

domain.setReplayGiveUpDelay(long) is gone from master - #901 replaced it with the replay-give-up-delay property - so the test did not compile after the rebase. It takes the route the other cases of this class take since that commit: unlimited while it sets up, so the change whose replay fails keeps holding the barrier, 2000ms once the parked change is back so that it is given up on, and the property removed in the finally.

The ordinals

319 and 320 went into master with #945 while this sat, which is where replication.properties conflicted. 318 sits between them and is nobody else's, so it stays; #981 has moved on to 325.

Runs

UpdateOperationTest 30/30, RemotePendingChangesTest 24/24, AssuredReplicationPluginTest 14/14, ModifyConflictTest 36/36, NamingConflictTest 7/7, StateMachineTest 5/5, DependencyTest 3/3 - 119 tests, Skipped: 0. NamingConflictTest needs a run of its own: in one pass with the others its setUp loses the administration port to the server the previous class left behind.

The description is updated to match.

vharseko · 2026-09-11T18:36:08Z

Rebased onto the head of #958, which is rebased onto master. That branch grew a fifth commit, [#922] Pin the ownership index on the hand-out and the give-back, and assert the alert an OutOfMemoryError leaves behind; the fix itself is unchanged, what follows is what the rebase had to decide.

The javadoc of the ownership index

It conflicted where both branches rewrote it. This PR's sentence stays - a parked change is given back by releaseParkedChangesOwnedByCurrentThread(), which reads the changes which are waiting rather than the index - and the paragraph the fifth commit added below it, on who writes each entry of the index, stays with it: the give-back of the parked changes does not touch the index.

One comment the fifth commit made untrue

The OutOfMemoryError arm of the ack said the give-back on the way out of replay() is a no-op once the change was committed, and that the changes parked behind it wait for the next replay of this domain to hand them out. True of the change the thread was replaying; not of the give-back as a whole with this PR: the changes this thread parked are handed back and the session is restarted for them, the ones other threads parked wait for getNextUpdate(). The comment says so now.

The barrier of the end-to-end test

It was ModifyMsgWithAnUnparseableOperationDN, and #928 - in master since - has a modify whose entry DN does not parse reported once and recorded as replayed rather than asked for again: nothing would have been parked behind it. The barrier is now ModifyMsgWhoseOperationRefusesAControl, the delivery which is built and then refused and so stays listed and uncommitted - the move #958 made for its own test on that merge.

Ordinals

318 is still nobody's: master holds 319, 320, 326 and 327.

Runs

UpdateOperationTest 32/32, RemotePendingChangesTest 24/24, AssuredReplicationPluginTest 14/14, ModifyConflictTest 36/36, StateMachineTest 5/5, DependencyTest 3/3 - 114 tests in one pass - and NamingConflictTest 8/8 in a run of its own; 122 tests, Skipped: 0.

The description is updated to match.

vharseko · 2026-09-12T13:19:57Z

Rebased onto master now that #958 is merged. Nothing had to be decided this time: the head of #958 this sat on is the tree master took in 776339a, so the one [#954] commit went over without a conflict, and the tree of this head is the tree of the previous one, 48420fa - the one CI passed in full. The branch is that one commit on top of master now.

Ordinals: 315, 316 and 317 are in master with #958; 318 is still only this PR's.

The description says "built on #958" instead of "stacked on #958" and names the master commit it is in.

maximthomas

praise: The give-back is built the way the file's own rules ask for, and the dangerous part — a released change being handed out twice — is closed.

releaseParkedChangesOwnedByCurrentThread() removes the change from dependentChanges and drops its owner in one step under pendingChangesWriteLock → dependentChangesLock; that is the lock order of clear() (:494-495) and getNextUpdate() (:683-684), and getNextUpdate() re-checks under the same lock (:686), so a released change can not be handed out as a dependency and redelivered at once.
The redelivery road works as claimed: putRemoteUpdate() (:231-238) accepts a change which is listed, uncommitted and unowned, and markInProgress() takes it.
The last-resort restart moved outside the owned != null guard (LDAPReplicationDomain.java:2702-2711), with the reason written next to it; ordinal 318 is free on master and in every open PR.

issue (blocking): The give-back un-parks a change before the allocation which can fail, and the restart is requested only after the give-back returns.

opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/RemotePendingChanges.java:624-633, opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/LDAPReplicationDomain.java:3729-3739, :2682-2685

The loop runs it.remove(); change.setOwner(null); and then released.add(change.getCSN()) into a new ArrayList<>() with no initial capacity: the first add is an allocation. giveBackParkedChanges() sets sessionRestartRequested only once the release has returned, and the last-resort catch requests a restart only under owned != null. On the road :3197-3204 names — the replay committed (commit() cleared owner and index), then an OutOfMemoryError past the ack — owned == null; if the add throws, the change is out of dependentChanges, owner-less, and no restart is requested. Only the restart some later failed replay asks for delivers it. At BASE it stayed parked and the next replayed change handed it out through getNextUpdate(). This is the allocation-before-mutation rule getNextUpdate() states for itself at :690-695.

// RemotePendingChanges.releaseParkedChangesOwnedByCurrentThread(), :624
// the one allocation of this method, made before anything is taken out of the set
final List<CSN> released = new ArrayList<>(dependentChanges.size());

// LDAPReplicationDomain.replay(), last resort, :2669
catch (Throwable recoveryFailure)
{
  // whichever road released a change, before anything else here can throw
  sessionRestartRequested.set(true);
  if (owned != null)
  {
    remotePendingChanges.replayFailed(owned);
    ...

Pin: see the next issue — the case which turns red under this fix reverted is the one which exercises this road.

issue (blocking): The restart the give-back asks for, and its run on the parked road, are pinned by no case: the mutant with both deleted is green 1/1.

opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/LDAPReplicationDomain.java:3739, :2659-2668, opendj-server-legacy/src/test/java/org/opends/server/replication/UpdateOperationTest.java:3311

Measured: aChangeParkedByAnUnwoundReplayIsDeliveredAgain is green at head (1/1) and stays green (1/1) with sessionRestartRequested.set(true) at :3739 and runRequestedSessionRestarts(!replayThreadShutdown.get()) at :2667 deleted. Two reasons. ModifyMsgWhoseReplayIsUnwoundAfterItsAck throws from getCSN() at :3282, on the failed-change arm, with the change still owned — so recoverFromReplayFailure() restarts the session on the owned road whatever the give-back asked for. And the case redelivers the three changes itself (domain.processUpdate(...) in the repeatUntilSuccess loop, :3400-3420), so no restart is observed at all: the case pins the release (dependent-changes-size → 0) and nothing else. The description says the getNextUpdate() road "is the one the end-to-end test drives"; the test drives the :3282 arm. The fix above lands on exactly these lines, unpinned.

Pin: an unwind which leaves owned == null — a fixture whose replay commits and which throws from what replay() reads of it past the ack, the way this one does before commit() — and a redelivery which only a new session brings: publish the parked change through a broker to the replication server instead of processUpdate(), drop it from the case's own loop, and keep

checkEntryHasAttributeValue(waitedOn, "description", parkedDescription, 30,
    "the change which was parked must be applied by the delivery which took it over");

Red under the :3739 + :2667 mutant is the check; amend the description's sentence about the road the end-to-end test drives.

issue (non-blocking): The parked-road restart sits through the backoff on an OutOfMemoryError, against the policy the owned road applies.

opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/LDAPReplicationDomain.java:2667, policy at :2641-2647 and :3632

Reached when owned == null, or the owned change was given up on, and t is an OutOfMemoryError: a thread on its way out sleeps up to 10 s holding replayFailureRecovery, the #923 alert is delayed and consecutiveSessionRestarts is bumped.

runRequestedSessionRestarts(!replayThreadShutdown.get() && !(t instanceof OutOfMemoryError));

suggestion (non-blocking): The processed count and message 318 are asserted by no case.

opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/LDAPReplicationDomain.java:3740-3744

"Counted as processed, not against the failure counter" holds by reading only: deleting incProcessedUpdates() and the logger.info is green. One assert in the end-to-end case covers the count; the message is pinned the way the other NOTE lines of this class are, or left as is.

final long processed = getMonitorAttrValue(baseDN, "replayed-updates");
...
assertMonitorAttrValueEventually(baseDN, "replayed-updates", processed + 1,
    "a change given back must be counted as processed");

nitpick (non-blocking): Message 318 says the replication server "sends it again" on the road where nothing will.

opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/LDAPReplicationDomain.java:2627, :3739-3744, :3761

giveBackParkedChanges() runs before the replayThreadShutdown || shutdown || disabled check, so on the abandon road — where abandonReplay() returns without asking for anything — the flag is set, the count bumped and the line logged all the same. The stale flag is closed by disable() (clear :4505 before set(false) :4517), so this is the text and the count. Either pass the road in and skip the report and the count on it, or word 318 so that it holds ("…and is delivered again by the next session of this domain").

nitpick (non-blocking): The @return of releaseParkedChangesOwnedByCurrentThread() describes a per-replay scope the method does not have.

opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/RemotePendingChanges.java:608-610

"…which is what every replay which was not unwound while it held one leaves behind" — a replay which parks a change and returns leaves the thread owning it (addDependency() :753 clears the index, not the owner). The method gives back every change the thread parked and has not committed, as the body says.

 * @return the CSNs of the changes it gave back, oldest first; empty when this thread owns
 *         no parked change - the changes a thread parked stay its own until they are
 *         committed or given back, whichever replay parked them

nitpick (non-blocking): Two resets in one finally, unnested.

opendj-server-legacy/src/test/java/org/opends/server/replication/UpdateOperationTest.java:3431-3432

A throw from resetReplayGiveUpDelay() leaves the pool at one thread for every class after this one in the failsafe JVM. Pre-existing pattern in the file; this case is the one which sets both.

finally
{
  try { resetReplayGiveUpDelay(); }
  finally { resetNumUpdateReplayThreads(); }
}

nitpick (non-blocking): The changes-in-progress-size steps are satisfied at listing time, not at the replay they name.

opendj-server-legacy/src/test/java/org/opends/server/replication/UpdateOperationTest.java:3359-3361, :3380-3382

putRemoteUpdate() is synchronous, so inProgress + 1 and inProgress + 3 are reached as the change is listed, before its replay runs or fails; "the replay … must have been reached" overstates what is waited for. The case is red without the give-back regardless — text only.

vharseko · 2026-09-16T07:26:45Z

Rebased onto master - nine commits since this was last pushed, LDAPReplicationDomain among them (#926, #952, #953); the one [#954] commit went over clean and compiles. Then the round.

The allocation comes first, the request comes first

releaseParkedChangesOwnedByCurrentThread() sizes its list for every change which is waiting, before anything is taken out of the set: that is the one allocation of the method now, and the rule getNextUpdate() states for itself holds here. The last resort of replay() asks for the restart as its first line and whichever road threw - it can not throw, so no road out of that catch is left without the request - and runs it outside the owned != null guard as before.

The end-to-end test pins the restart the give-back asks for

Measured the same way: the case at the previous head was green with sessionRestartRequested.set(true) in giveBackParkedChanges() and the runRequestedSessionRestarts() of the parked road both deleted, for the two reasons named.

owned == null past the ack is reachable by one road only. On the road of an applied change replay() reads nothing of the message once the ack is out - replayFailed is false, so msg.getCSN() is not evaluated, and what follows is getNextUpdate() - and inside processUpdateDone() every throw but an OutOfMemoryError is caught as the ack which could not be published (#958). So the fixture is ModifyMsgWhoseAckRunsOutOfMemoryOnceApplied: a plain ModifyMsg whose isAssured() throws the error, the sibling of ModifyMsgWhoseAckRunsOutOfMemory on the road which commits. The replay leaves replay() with its own change in the data and owned by nobody, the give-back finds the parked change alone, and the restart it asks for is the one which is run.

The parked change is published through a broker of its own (server id 30) rather than handed to the domain, so nothing but a new session brings it back; the case's own loop delivers the barrier alone. What it watches, in order: dependent-changes-size to 0 - the give-back; the thread which met the error is gone (#923) and it was the whole pool, so the delivery the restarted session brings waits in the replay queue and that reading is not a window; the pool brought back, and dependent-changes-size to 1 - the change parked again behind the barrier, which is what says a new session delivered it; the barrier given up on under a budget of 0ms - a change given up on asks for no restart - and the parked change applied. Red under the mutant on the second reading: expected [1] but found [0], after 60 s of nothing being delivered. Red without the give-back on the first, as before.

The parked road runs the restart without the backoff on an OutOfMemoryError, the way the owned road does - the case runs on that road, and a thread which is ending is not one to keep for up to ten seconds.

The count and the line

The count is not observable by a monitor read: the road which bumps it restarts the session in the same breath, and every reconnection - connectAsDataServer() → toNotConnectedStatus() → resetMonitoringCounters() - zeroes replayed-updates. The +1 is gone before a test could read it, so it stays as what it is, bookkeeping which keeps the counter true until the session it belongs to is over. Message 318 is pinned the way the other NOTE lines of the replay roads are: none of them is asserted by any test in the tree, so it is left as is.

The abandon road

The give-back mirrors abandonReplay() there: on a domain which is going away or is being imported into the changes are released and nothing more - no request, no count, no line. The domain owns its session and forgets its pending changes on its way down, and a line per change saying the replication server sends it again would not hold before the server is started back. 318 keeps its wording, since it is now written only where it is true.

Text

The @return of releaseParkedChangesOwnedByCurrentThread() says what the method does: the changes a thread parked stay its own, whichever replay parked them, until getNextUpdate() hands them out or they are given back here. The two finally blocks of the case are nested, and the broker has one of its own. The waits which are satisfied at listing time say so.

Runs

UpdateOperationTest 32/32, RemotePendingChangesTest 24/24, AssuredReplicationPluginTest 14/14, ModifyConflictTest 36/36, StateMachineTest 5/5, DependencyTest 3/3 - 114 tests in two passes - and NamingConflictTest 21/21 in a run of its own; 135 tests, Skipped: 0. The new case, with the request of the give-back and the run of the parked road deleted: 0/1, the change which was given back must be delivered again by the session which was restarted for it expected [1] but found [0].

The description is updated to match.

maximthomas

praise: The give-back now holds on the road it was written for, and the case proves it rather than describes it.

releaseParkedChangesOwnedByCurrentThread() sizes released (opendj-server-legacy/src/main/java/org/opends/server/replication/RemotePendingChanges.java:633) before the first it.remove() (:640): nothing is un-parked ahead of an allocation.
ModifyMsgWhoseAckRunsOutOfMemoryOnceApplied throws out of isAssured() once the commit is in, so replay() unwinds with owned == null — the road round 1 asked for — and the parked change travels the RS (serverId 30) to come back only through a new session.
Measured here, not only read: aChangeParkedByAnUnwoundReplayIsDeliveredAgain green 1/1 at head (4.0 s); with the request (LDAPReplicationDomain.java:3925) and the run (:2744-2745) deleted, red 0/1 at UpdateOperationTest.java:3421 after the 60 s budget — your 0/1 reproduced.
The last resort asks first and unconditionally (:2767) and runs outside the owned guard (:2791); the nested finally stops the broker before the pool is put back.

suggestion (non-blocking): The no-backoff term on the OutOfMemoryError road is pinned by no case.

opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/LDAPReplicationDomain.java:2744-2745

Measured: with && !(t instanceof OutOfMemoryError) dropped the case is green 1/1 and 1.4 s slower — the dying thread sleeps the first backoff (1 s, :4024) and the 60 s wait at UpdateOperationTest.java:3421 absorbs it. consecutiveSessionRestarts moves only under wait (:3989): it is what only the backoff road produces.

// LDAPReplicationDomain, next to :6430
attributes.add("consecutive-session-restarts", consecutiveSessionRestarts.get());

// UpdateOperationTest, around :3408-3421
final long restarts = getMonitorAttrValue(baseDN, "consecutive-session-restarts");
deliverUntilMonitorReaches(domain, "dependent-changes-size", 0, /* as today */);
setNumUpdateReplayThreads(2);
assertMonitorAttrValueEventually(baseDN, "dependent-changes-size", 1, /* as today */);
assertEquals(getMonitorAttrValue(baseDN, "consecutive-session-restarts"), restarts,
    "a thread an OutOfMemoryError is ending must not wait the backoff out");

issue (non-blocking): giveBackParkedChanges() does not take the import road its javadoc, the description and your reply name.

opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/LDAPReplicationDomain.java:3915, :3898-3906

The early return is gated on shutdown.get() || disabled (= ownsItsSession(), :5713) where abandonReplay (:3947), recoverFromReplayFailure (:3762) and restartSession (:3971) gate on sessionHasAnOwner() (:5736, adds importInProgress()). importingData = true (:5356) precedes the bounded awaitReplayDrained() (:5357), so a replay unwound inside that window gives back, asks for a restart restartSession() refuses (the request is spent), logs one NOTE_REPLAY_PARKED_CHANGE_GIVEN_BACK per change which no session will send again, counts them as processed, and a non-OOME unwind sleeps the backoff at :2744. The end state is the sibling road's — the import's finally (:5438-5441) clears — hence non-blocking; the @return at :3906 ("so that the caller restarts") is false on that arm too.

if (sessionHasAnOwner())
{
  // The domain owns its session, or an import does: both forget the pending changes.
  return true;
}

issue (non-blocking): The second lock() sits outside the try in a method which runs on the OutOfMemoryError road by design.

opendj-server-legacy/src/main/java/org/opends/server/replication/RemotePendingChanges.java:616-618

A contended ReentrantLock.lock() allocates its queue node; a throw out of dependentChangesLock.lock() (:617) unwinds past :616 with pendingChangesWriteLock held by a thread which is ending, and putRemoteUpdate (:221), commit (:268), replayFailed (:337) block for good. Six pre-existing sites share the shape (:180, :494, :666, :692, :738); this one is new and on the targeted road.

pendingChangesWriteLock.lock();
try
{
  dependentChangesLock.lock();
  try
  {
    // :620-645 unchanged
  }
  finally
  {
    dependentChangesLock.unlock();
  }
}
finally
{
  pendingChangesWriteLock.unlock();
}

suggestion (non-blocking): The abandon arm of the give-back is reached by no case.

opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/LDAPReplicationDomain.java:3915-3918

No UpdateOperationTest case disables or shuts the domain down while an unwinding thread owns a parked change, and RemotePendingChangesTest has no domain: the early return (round-1 M2) deleted leaves every case green.

Pin: park a change as the new case does, set ds-cfg-enabled: false on the domain before the owning replay is unwound, and assert that replayed-updates did not move and no NOTE_REPLAY_PARKED_CHANGE_GIVEN_BACK was logged — both are what only the request arm produces.

issue (non-blocking): The last resort's comment justifies the unconditional request with a road which does not exist.

opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/LDAPReplicationDomain.java:2760-2767, :2791

"a throw out of it before that may have left changes which nobody owns anymore": after the release nothing in giveBackParkedChanges() can throw (unlock → isEmpty → two volatile reads → set). Everything which can — the lock() node allocations, the sized list (RemotePendingChanges.java:633), iterator() (:634) — throws before any release, and the changes stay parked and owned, as at the base. The sizing closes round-1 [1] by construction, which no case can observe, and nothing reaches :2767/:2791 (no case injects a throw into the catch since #958). Reword to the road the request does cover — if that is a give-back which threw before releasing anything, say so; a case injecting that throw would then reach the last resort for the first time.

suggestion (non-blocking): resetNumUpdateReplayThreads() asserts inside a finally.

opendj-server-legacy/src/test/java/org/opends/server/replication/UpdateOperationTest.java:3518-3525, :3458-3461

A delete which fails there replaces the case's own exception: the failure the case was written for is reported as "the number of replay threads could not be put back".

/** Returns the result code, so that a finally can call it without masking a failure. */
private static int resetNumUpdateReplayThreads() throws Exception
{
  return TestCaseUtils.applyModifications(true,
      "dn: " + SYNCHRO_PLUGIN_DN,
      "changetype: modify",
      "delete: ds-cfg-num-update-replay-threads");
}

Assert the code on the case's normal road (last statement of the try at :3362); the finally calls it best-effort.

suggestion (non-blocking): A red before the barrier is given up leaves failing listed for the rest of the class.

opendj-server-legacy/src/test/java/org/opends/server/replication/UpdateOperationTest.java:3431-3449, :3463-3465 (same shape in the sibling :2152-2230)

failing was handed to the domain and never travelled the RS: only the cover loop at :3436-3449 gives it up. The outer finally puts the delay back but not the change, so after an earlier failure hasFailingChanges() stays true and every later session restart in the class waits the backoff out (up to 10 s).

finally
{
  if (!domain.getServerState().cover(failing))
  {
    giveUpOn(failing, waitedOn, failingMods, waitedOnUUID); // :3431-3449, extracted
  }
  resetReplayGiveUpDelay();
}

vharseko · 2026-09-16T11:10:36Z

Rebased onto master at c6b483127f, which has #981 in it - [#925] Keep asking for a session restart until it has run - and #981 rewrote the road this PR's give-back asks its restart on: the sessionRestartRequested flag is SessionRestartRequests now, the wait belongs to the request rather than to the thread which runs it, and a request nobody runs is run by the state checkpointer. The one [#954] commit goes over it with two conflicts, LDAPReplicationDomain and replication.properties, and the first is not textual. What the rebase had to decide:

The give-back asks the way the owned road asks

giveBackParkedChanges(SessionRestart) makes sessionRestarts.request() where it set the flag - still before the changes are reported, so a report which can not be formatted loses the report and not the request. What it asks for is what recoverFromReplayFailure() asks for on the same thread: AFTER_BACKOFF, or NOW on a thread which is stopping or which an OutOfMemoryError is ending. That is where the no-backoff term of the parked road went, since runRequestedSessionRestarts() takes no argument anymore.

A thread which is stopping leaves the restart standing

The parked road ran runRequestedSessionRestarts(false) on a stopping thread. #981's abandonReplay() deliberately does not: the threads of the pool are stopped one after the other and joined, and each running a restart on its way out would have the configuration change wait for one restart per thread, so it asks and leaves the one restart to the checkpointer. A parked road which ran what is requested on that thread would have run abandonReplay()'s request with it, so it is gated on !replayThreadShutdown.get() now and leaves it standing the same way. The other two roads are what they were: a thread whose replay was unwound runs the restart at once, with the backoff; a thread an OutOfMemoryError is ending runs it at once, without.

The last resort

sessionRestarts.request(NOW) is its first line, whichever road threw, and the run is outside the owned != null guard as before. The wording around it is #981's - a restart which can not run there leaves its request standing, and the checkpointer runs it - with this PR's paragraph on why the request comes first kept under it.

Text

317 is master's wording ("a restart of the session is asked for"), 318 is this PR's. The description says the above where it said the parked road runs without the backoff on two roads, and that 325 is master's with #981.

Runs

On this head: UpdateOperationTest 34/34 in a run of its own - in one pass with the others its setUp lost the administration port to the server the previous class left behind - and RemotePendingChangesTest 24/24, SessionRestartBackoffTest 3/3, DependencyTest 3/3 in one pass; 64 tests, Skipped: 0. The mutants are not re-measured on this head.

This push is the rebase; the non-blocking items of the approving round are not taken in it.

maximthomas

PR #985 — review feedback, round 3 (head `42284c5`)

Review action: Request changes — one blocking issue the rebase over #981 created (the last resort's first statement can throw on the OOME road); the rest is non-blocking, mostly carried from round 2.

praise: The rebase over #981 keeps the design honest, and the new case pins the mechanism that matters.

The give-back asks NOW for a stopping or OOME thread and AFTER_BACKOFF otherwise (opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/LDAPReplicationDomain.java:2752-2754), and a stopping thread leaves its request to the checkpointer the way abandonReplay() does.
aChangeParkedByAnUnwoundReplayIsDeliveredAgain pins the request: with sessionRestarts.request(restart) (:4033) deleted it is red 0/1 at opendj-server-legacy/src/test/java/org/opends/server/replication/UpdateOperationTest.java:3688 ("expected [1] but found [0]"). The unwound replay meets a real OutOfMemoryError (ModifyMsgWhoseAckRunsOutOfMemoryOnceApplied), so the OOME road is the road under test.
The description says up front which round-2 items are not taken and that the mutants were not re-measured on this head.

issue (blocking): sessionRestarts.request(SessionRestart.NOW) as the last resort's first statement can throw on the OOME road, and then replayFailed(owned) never runs.

opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/LDAPReplicationDomain.java:2818-2829, opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/SessionRestartRequests.java:119-122

request() is merge(): accumulateAndGet with an inline lambda. The lambda's invokedynamic site and the VarHandle site inside AtomicReference.weakCompareAndSetVolatile are both linked on their first execution in the JVM, and linking allocates. Nothing executes them at boot — every caller of request() is a replay-failure road — so on a JVM whose first restart request is this last resort, the request throws OutOfMemoryError again: replayFailed(owned) is skipped (the change stays owned by the dead thread, issue #922), the released parked changes have no request standing, and the linkage error replaces t. At c6b483127f the request stood after replayFailed(owned), so the same throw cost the restart, not the hand-back. Measured with a probe on a packed heap (JDK 11/17/21/26, 2/2 each): merge() as written throws; a static final BinaryOperator still throws (the VarHandle site); merge() executed once at construction runs. "It can not throw" (:2823) holds only after that first execution.

// SessionRestartRequests
SessionRestartRequests()
{
  // Links the lambda and the VarHandle sites now, on a thread which can allocate: every
  // request is made on a replay-failure road, where an allocation may be what has just failed.
  merge(SessionRestart.NONE);
}

Or: put remotePendingChanges.replayFailed(owned) back first, under its guard, and the request after it, as at c6b483127f — a request which throws then costs the restart, not the hand-back. Both is best.

suggestion (non-blocking): The parked-road runRequestedSessionRestarts() is pinned by no case.

opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/LDAPReplicationDomain.java:2804

With the call deleted the case is green 1/1 (4.14 s against 3.87 s): the checkpointer runs the standing request within its 1 s tick, inside the 60 s budget of UpdateOperationTest.java:3688. The comment (:2795-2803) presents the run as load-bearing; today it is a latency optimisation. Pin: domain.failNextSessionRestarts(1) before the unwound change, then assert ERR_REPLAY_SESSION_RESTART_FAILED (325) is not logged — only the checkpointer reports that failure (:3968), so the case is red once the replay thread stops running the restart itself.

issue (non-blocking): pendingChangesWriteLock.lock(); dependentChangesLock.lock(); before the try leaks the write lock when the second lock() throws.

opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/RemotePendingChanges.java:616-618

A contended ReentrantLock.lock() allocates its queue node, and releaseParkedChangesOwnedByCurrentThread() runs on the OOME road by design. A throw out of the second lock() leaves the first held by a dying thread: putRemoteUpdate (:221), commit (:268) and replayFailed (:337) block for good. The class's other double-lock sites have the same shape — pre-existing there, new on this road.

pendingChangesWriteLock.lock();
try
{
  dependentChangesLock.lock();
  try
  {
    ...
  }
  finally
  {
    dependentChangesLock.unlock();
  }
}
finally
{
  pendingChangesWriteLock.unlock();
}

issue (non-blocking): giveBackParkedChanges() refuses on shutdown.get() || disabled where the restart it asks for refuses on sessionHasAnOwner().

opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/LDAPReplicationDomain.java:4023-4027, :4088

sessionHasAnOwner() adds importInProgress(). importingData = true (:5625) precedes the bounded awaitReplayDrained() (:5626), so a replay unwound inside the import window gives its parked changes back, logs one NOTE 318 per change, counts them as processed and asks for a restart which restartSession() refuses at :4088 — spent, nothing gives it back. Nothing is lost (the import's finally clears and restarts, :5708), but the log and the counter announce a delivery which is not coming. The javadoc and the round-2 reply claim the import road.

if (sessionHasAnOwner())
{
  return true;
}

suggestion (non-blocking): The NOW arm of the ternary is pinned by no case.

opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/LDAPReplicationDomain.java:2752-2754

The case drives this road (the unwound change OOMEs in its ack), but with || t instanceof OutOfMemoryError dropped the give-back asks AFTER_BACKOFF, the owned road's NOW merges into it (merge() keeps the greater ordinal), and the dying thread sleeps its backoff (:4106) — at most 10 s, inside the 60 s budget: green by reading. Pin: measure from the give-back (NOTE 318) to the second dependent-changes-size reading with a budget below REPLAY_RETRY_DELAY_IN_MS (1 s); only the NOW arm is that fast.

suggestion (non-blocking): No case stops the pool, or the domain, with a parked change owned by an unwinding thread.

opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/LDAPReplicationDomain.java:2789 (the !replayThreadShutdown.get() term), :4023-4027 (the early return)

Both are reached by nothing: the pool reconfigurations of the e2e case (UpdateOperationTest.java:3628, :3687) happen with nothing parked, and no case disables or shuts down with a parked change. Dropping the term, gating everything on it, or deleting the early return: green by construction. Pin: one case which parks a change on a single replay thread and changes ds-cfg-num-update-replay-threads while it is parked, asserting the change is delivered again and that the checkpointer ran the restart (ERR 325 under failNextSessionRestarts(1)); one which disables the domain in the same state, asserting no NOTE 318.

nitpick (non-blocking): Three comments describe a road the code does not take.

opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/LDAPReplicationDomain.java:2747-2750, :2810-2812, :2818-2824

:2747-2750 "runs it at once, without [the backoff]": merge() keeps the greater ordinal, so NOW never overrides a standing AFTER_BACKOFF — the give-back's own, left by :3905, or another thread's. "Asks for it without the backoff" is what holds.
:2810-2812 "leaves its request standing, and the state checkpointer of this domain runs it": true on the throw road (:3905) and when the CAS is lost; on the import road take() (:3885) then :4088 returns and nothing gives the request back.
:2818-2824 "it can not throw": see the blocking issue.

nitpick (non-blocking): setReplayGiveUpDelay("0ms") is set after three waits.

opendj-server-legacy/src/test/java/org/opends/server/replication/UpdateOperationTest.java:3698

A red before it leaves failing listed under an unlimited give-up budget for the rest of the class: hasFailingChanges() stays true and every later session restart sleeps up to 10 s. Give the change up in the outer finally, or set the delay before the first delivery.

nitpick (non-blocking): resetNumUpdateReplayThreads() asserts inside a finally.

opendj-server-legacy/src/test/java/org/opends/server/replication/UpdateOperationTest.java:3727, :3785-3792

A failing delete replaces the case's own exception. Return the result and assert it in the case body, or add the failure as suppressed.

vharseko · 2026-09-16T14:25:07Z

The round on 42284c5, and with it the non-blocking items of the approving round, which it carries. The head before this one, c1403da0, went out with the round-2 items before I had read this review; what follows is the whole of it, on this head.

The request which can throw on its first execution

Both fixes, as you suggest. SessionRestartRequests runs merge(NONE) - and take(), which is run on the same roads - once in its constructor, on a thread which can allocate, so that the lambda's call site and the VarHandle sites are linked before any request is made on a replay-failure road. And the last resort's request is back where #958 made it, after replayFailed(owned), so that a request which throws costs the restart and not the hand-back. That order is also the one every other road which releases a change keeps: asked for before the release, a request could be taken and run by another thread while this one still owned the change, and the delivery the new session brought would be turned down as the duplicate of a change a replay thread owns, with nothing left standing to ask for it again. What stays outside the guard is the run, for the two roads which reach the last resort with a request standing and no change of this thread's: a give-back which released, asked and then threw reporting, and a restart the parked road ran and which threw. The comment says that, and no longer says the request can not throw.

The give-back's owner road, and the last resort's

giveBackParkedChanges() gates on sessionHasAnOwner(), the way abandonReplay(), recoverFromReplayFailure() and restartSession() do, and so does the parked road of replay() which runs what is requested: a request another thread left standing is not this one's to spend on a restart which is refused where it runs. The last resort's run is gated the same way now, which is what makes its comment true on the import road as well - the request stands there rather than being spent on the refusal, and the checkpointer holds it for as long as ieRunning(), from the request to the end of the import, then runs it or finds it cleared with the pending changes.

Pinned in ReplayDuringImportTest, on the import road: it is the one road with an owner a test holds open for as long as it needs - the request is on its way until the exporter answers it - while disable() forgets the pending changes a moment after it takes the session; the ack which unwinds the replay is published outside the replay lock, so disable() is not held off by the replay which is unwinding and the give-back races remotePendingChanges.clear(), and disable() clears the request and zeroes the counters on its way, so on that road the request arm leaves nothing to read. On the import road the barrier's attempts end on an entryUUID search which does not run - the shape of the sibling case - the change which follows it on the same DN is parked by the thread of the test, and that thread's replay of an applied change is unwound by the OutOfMemoryError fixture, caught where a replay thread would have ended. What is read: dependent-changes-size to zero, replayed-updates unmoved - no session restart happens on that road, so the count is not zeroed under the reading, which is what made it unreadable in the end-to-end case - no NOTE 318 for the parked change, and the session the answer arrives over still up; the exporter then answers, and the import runs to its end. Red with the owner road deleted and red under shutdown || disabled: 0/1 both, a change released while a total update owns the session must not be counted as processed ... expected [1] but found [2], and 318 is in the error log of both runs.

The parked road's run, and the stopping thread's

The comment presented the run as load-bearing; it says what it is now - the latency of the delivery the changes wait for, since the checkpointer runs a request left standing within its tick - and both terms are pinned in a class of their own, ParkedChangeGiveBackTest, on the fixture of ReplayDuringImportTest: the replay runs on the thread of the test, which is also what says whether that thread is stopping - the flag it passes to replay() - and the restart is asked to fail once, failNextSessionRestarts(1), so that the thread which ran it is the one which met the failure.

theThreadWhichGaveBackAParkedChangeRunsTheRestartItAskedFor: the injected failure comes out with the OutOfMemoryError, as a throwable it suppressed - the thread ran the restart itself, met the failure, and the last resort ran the restart again, so the session is back before replay() returns. Pinned on the suppressed throwable rather than on 325's absence: 325's absence races the checkpointer's tick in the instants between the request being made and being taken, the window the case at UpdateOperationTest.java:2430 tolerates with its "once, or twice"; the same window exists here in the other direction, and is as narrow. Red with the run deleted: 0/1, Expected size: 1 but was: 0 on the suppressed throwables.
aStoppingThreadLeavesTheRestartItAskedForToTheStateCheckpointer: nothing of the failure comes out with the error, and the checkpointer reports it (325) within its tick, then brings the session back after the backoff. Red with the !replayThreadShutdown.get() term dropped: 0/1, Expecting empty but was: [IllegalStateException: the session of domain o=test could not be started again, as a test asked].

The pool being reconfigured with a change parked on the thread it stops is #988's case, and the domain being disabled in that state is the race above.

The no-backoff term on the OutOfMemoryError road

Pinned through getConsecutiveSessionRestarts(), which #981 exposes for its own tests, rather than by timing: read before the replay is unwound and once the change is back, equal. A restart which waits its backoff out is the one road which moves that count, and nothing else can move it in between - a replay which made it puts it back to zero only once nothing is failing anymore, and the barrier keeps failing until it is given up. Red with the give-back asking with the backoff on every road: 0/1, expected [1] but found [2].

The second lock

Taken inside the try of the first, with the reason next to it. The older sites are left as they are.

The three comments

:2747 says "asked for without the backoff" and that what is run is what stands - a request is never answered by less than it asked for. :2810 holds now that the last resort does not run a restart a domain with an owner would refuse. :2818 is gone with the request's move.

The test's way out

resetNumUpdateReplayThreads() returns the result code; it is asserted as the last statement of the normal road, and the finally calls it best-effort. The barrier is given up on the way out of a red as well - giveUpOn() is the loop the normal road ran, extracted - and, that being a cleanup after a failure which is being reported, a throw out of it is logged rather than allowed to replace the failure.

Runs

On this head, one JVM per class: ParkedChangeGiveBackTest 2/2, ReplayDuringImportTest 5/5, UpdateOperationTest 34/34, RemotePendingChangesTest 24/24, SessionRestartBackoffTest 3/3, DependencyTest 3/3; 71 tests, Skipped: 0. The round-2 mutant is re-measured on this head as well - the request of the give-back and the run of the parked road deleted: 0/1, expected [1] but found [0] on the reading which says a new session delivered the change.

The description is updated to match.

maximthomas

praise: The round-3 blocker is closed the right way, and the new test class pins what round 2 measured unpinned.

SessionRestartRequests() runs merge(NONE); take(); once at construction, so the lambda site and both VarHandle sites are linked on the config thread and the OOME road's request(NOW) allocates nothing from then on.
The last resort hands the change back before it asks: replayFailed(owned) first, request(NOW) second, both under the owned != null guard.
ParkedChangeGiveBackTest kills the mutants round 3 could not: the parked-road run deleted and the !replayThreadShutdown.get() term dropped are red 0/1 on their own observable (the suppressed injected failure), not on the checkpointer's.
releaseParkedChangesOwnedByCurrentThread() takes the second lock inside the first lock's try, and replayFailed(csn) is owner-guarded, so a second hand-back of the same CSN is a no-op.
The five Linux CI cells are green at this head with UpdateOperationTest running beside every other class.

question (non-blocking): The give-back under an owner requests nothing, while the last resort under an owner leaves its NOW request standing and gates only the run.

opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/LDAPReplicationDomain.java:4056-4061, :2833-2836, :2874-2877

On the total update which never begins (initializeFromRemote refused or timed out) releaseIEContext() only nulls the context, and none of the three sessionRestarts.clear() sites (disable :5061, enable :5228, import finally :5742) is on that road. A standing request would be run by the state checkpointer within its tick, and the RS would send the released changes back; with nothing standing they sit unowned until the next failed replay, which is the hold-back the javadoc at :4030-4040 and :6030-6036 documents. Was that hold-back chosen, or is the asymmetry accidental? If accidental:

    if (sessionHasAnOwner())
    {
      // The owner forgets the pending changes; the request outlives it on the road
      // which clears none (a total update which never begins), and is cleared on the
      // others: the import's finally, disable(), enable().
      sessionRestarts.request(restart);
      return true;
    }

suggestion (non-blocking): aStoppingThreadLeavesTheRestartItAskedForToTheStateCheckpointer runs the abandon road, not the applied road its helper and fixture describe.

opendj-server-legacy/src/test/java/org/opends/server/replication/plugin/ParkedChangeGiveBackTest.java:156-170, :229-233
opendj-server-legacy/src/test/java/org/opends/server/replication/plugin/ModifyMsgWhoseAckRunsOutOfMemoryOnceApplied.java:49-52

With STOPPING set before the replay, goingDown is true at LDAPReplicationDomain.java:2995, the change is abandoned at :3232-3245 and never applied; the finally still calls processUpdateDone, whose first read is isAssured(), which throws unconditionally. The catch then sees owned still set: the give-back requests NOW and abandonReplay(owned) at :2759 requests NOW again. Two requests on one door — the give-back's stopping-road request deleted survives this case by construction (the abandon arm's request stands in, every assertion holds); the !replayThreadShutdown.get() term's mutant is killed as you measured. The javadoc "a change which is applied and whose ack runs out of memory" and the fixture's name are false for this case.

A fixture which stops the thread inside the ack reaches the give-back's stopping-road request with no abandon arm behind it:

  @Override
  public boolean isAssured()
  {
    // The pool stops this thread while its change is being acknowledged: applied and
    // committed under a running thread, unwound under a stopping one.
    stopping.set(true);
    throw new OutOfMemoryError("the ack of this applied delivery runs out of memory");
  }

Pin: the case passes a fresh new AtomicBoolean(false) to both the fixture and replayMsg; with the give-back's request deleted, nothing stands for the checkpointer and the wait for ERR_REPLAY_SESSION_RESTART_FAILED runs out.

note (non-blocking): The two new sessionHasAnOwner() gates on the run are reached load-bearing by no case.

opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/LDAPReplicationDomain.java:2791, :2874-2877

Under an owner the give-back requests nothing, so the term at :2791 guards a run that is a no-op either way; the last resort under an owner is reached by no case (ParkedChangeGiveBackTest case 1 and the UpdateOperationTest e2e case have no owner, ReplayDuringImportTest's import case takes the give-back's owner arm and does not throw). Both mutants survive by construction:

        if (parkedGivenBack && !replayThreadShutdown.get() && !shutdown.get() && !disabled)   // :2791, import forgotten

          runRequestedSessionRestarts();   // :2874-2877, gate deleted

A pin for :2791 needs another thread's request standing during the import (the last resort's, :2835); a pin for :2874 needs a throw inside the give-back on the import road. Your call whether either is worth a case.

vharseko · 2026-09-17T12:59:03Z

The approving round on 939fd02e9d, taken on 2d97f6c01c: the case of the stopping thread is changed, the other two are answered here.

The stopping thread is stopped by the ack

As you read it: with the flag set before the replay, goingDown is true at the top of the first attempt, the change is abandoned unapplied, and the OutOfMemoryError comes out of the processUpdateDone() the finally runs for it all the same - so the catch found the change still owned, and abandonReplay() asked for a restart of its own next to the give-back's. The case was green with the give-back's request deleted, because the abandon arm's request stood in for it; what the case's javadoc, the helper and the fixture's name said of it did not hold.

The fixture takes the flag now and sets it in isAssured() before it throws - the change is applied and committed under a running thread, and the thread is stopping by the time the replay is unwound - and the case hands one fresh flag to the fixture and to replay(). owned is null, no abandon arm runs, and the give-back's request is the one thing standing for the checkpointer. The other case and ReplayDuringImportTest take the constructor without the flag, as before.

Measured on this head, the give-back's request deleted: the case as it was, green 1/1; the case as it is, red 0/1 at the checkpointer's bound, the state checkpointer did not run the restart the stopping thread left standing: the failure that restart was asked to meet was never reported. The !replayThreadShutdown.get() term dropped, on the case as it is: red 0/1, a thread which is stopping must not run the restart it asked for - Expecting empty but was: [IllegalStateException: …as a test asked], as before. The class's javadoc, the case's and the helper's say which road is run and why the other would pin nothing.

The give-back under an owner asks for nothing: chosen, and the hold-back is master's

The give-back's owner arm is the arm of its two siblings, not of the last resort: recoverFromReplayFailure() and abandonReplay() both release under sessionHasAnOwner() and ask for nothing, and the road you name - a total update which is asked for and never begins - is the one #968 left standing and wrote into the javadoc of sessionHasAnOwner(), for the owned road. At #968 there was nothing to gain from asking under an owner: the request was a flag which only the thread of a failed replay ran, so a flag set under an owner was served by the next failed replay, which set it itself. The last resort asks regardless because it does not know which road threw and where; its request under an owner costs nothing - cleared by the two owners which forget the pending changes, run by the checkpointer for the one which does not.

Since #981 a request made under an owner is one the checkpointer runs within a tick of the owner going away, so the hold-back can be closed - but for the three roads as one policy. Taken in the give-back alone, a parked change released on that road would come back and the change recoverFromReplayFailure() released next to it would come back with it, while the same road with no parked change beside it - the common shape - would hold back as today, and the file would have two answers to one question. That is a change to what #968 decided on master's code, with a pin of its own (the request-on-its-way fixture of ReplayDuringImportTest, then abortStalledInitializeFromRemote() in place of the answer), so it is a follow-up: #1061. The javadoc of the give-back says the road it does not close, and the follow-up is where it is closed.

The two gates on the run

Not pinned in this round, on your call. :2791 becomes load-bearing on every owner road with the follow-up above - the give-back's own request then stands under the owner and a run there would spend it on restartSession()'s refusal - and that is where its pin goes, on the same fixture. :2874 is reached only by a throw inside the give-back on the import road, which no fixture reaches without a hook in the code under test; it stays as the mirror of the refusal restartSession() makes, with the comment saying so.

Runs

On this head, one JVM per class: ParkedChangeGiveBackTest 2/2, ReplayDuringImportTest 5/5 - the two classes which share the fixture; the other classes of the previous round are untouched by this one. The mutants above, and the parked road's run deleted against the case of the thread which runs its own restart: red 0/1 as before, Expected size: 1 but was: 0.

The description is updated to match.

maximthomas

praise: The stopping-thread case now pins what its javadoc says it pins, and says why the other road would not.

ModifyMsgWhoseAckRunsOutOfMemoryOnceApplied sets the flag inside isAssured(), after the commit: case 2 unwinds with owned == null, the give-back's request(NOW) is the one request standing, and the measurement shows it — request deleted: old shape green 1/1, new shape red 0/1; !replayThreadShutdown.get() dropped: red 0/1.
The fixture's javadoc states the abandon road and why a flag set before the replay pins nothing there.
The owner-road hold-back stays #968's policy and goes to #1061 for the three roads at once, instead of a second answer in one arm.

suggestion (non-blocking): Case 2 pins the give-back's request only while the fixture's flag flips after the apply; nothing in the case asserts the road it ran.

opendj-server-legacy/src/test/java/org/opends/server/replication/plugin/ParkedChangeGiveBackTest.java:170-187

Every assertion of the case holds on the abandon road too: the give-back (LDAPReplicationDomain.java:2754) runs before the owned check, the gate at :2791 is off on both roads, and abandonReplay() (:4086-4110) requests NOW just as the give-back does — your own measurement (old shape + request deleted: green 1/1) is that road. So the pin rests on the precondition the fixture's javadoc states ("nothing on the way in reads what throws here") and nothing asserts; a future read of msg.isAssured() before the apply moves the case back to the abandon road silently. replayed-updates-ok (numReplayedPostOpCalled, :2364) moves on the applied road only:

final long replayedBefore = getMonitorAttrValue(baseDN, "replayed-updates-ok");

final OutOfMemoryError unwinding = unwindAReplayWhichAppliedItsChange(other, true);

assertEquals(getMonitorAttrValue(baseDN, "replayed-updates-ok"), replayedBefore + 1,
    "the change must be applied and its replay unwound by the ack, not abandoned unapplied");

Mutant: the fixture in its previous shape (stopping set before replay()) — red on this line, green on every other assertion of the case.

suggestion (non-blocking): The abandon arm of replay()'s catch is executed by no case since case 2 moved off it.

opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/LDAPReplicationDomain.java:2757-2767, ParkedChangeGiveBackTest.java:170

Case 2 was the one case reaching owned != null && replayThreadShutdown.get() → abandonReplay(owned). The other replay(msg, flag) callers pass a flag that stays down (NamingConflictTest:1193, ReplayDuringImportTest:536, parkAChangeBehindABarrier), and every ReplayThread user in UpdateOperationTest runs on a live pool whose acks do not throw. Deleting :2767, or swapping it for recoverFromReplayFailure(owned, …), is green suite-wide. A third case in the previous shape keeps the arm pinned:

/** A thread stopped before its first attempt hands back the change it did not apply. */
@Test(timeOut = 120_000)
public void aStoppingThreadHandsBackTheChangeItDidNotApply() throws Exception
{
  parkAChangeBehindABarrier(addEntry("waitedOn"));
  final Entry other = addEntry("other");
  final int abandonedBefore = abandonedChangeReports().size();   // NOTE_REPLAY_ABANDONED_CHANGE, as restartFailureReports()

  final AtomicBoolean stopping = new AtomicBoolean(true);        // up before the replay: abandoned at the top of its first attempt
  replayMsg(new ModifyMsgWhoseAckRunsOutOfMemoryOnceApplied(gen.newCSN(), other.getName(),
      generatemods("description", "never applied"), getEntryUUID(other.getName())), stopping);

  assertEquals(abandonedChangeReports().size(), abandonedBefore + 1,
      "a stopping thread must hand back the change it did not apply");
  assertEquals(getMonitorAttrValue(baseDN, "dependent-changes-size"), 0);
}

Mutant: :2767 deleted — no NOTE_REPLAY_ABANDONED_CHANGE record (:4100), red.

suggestion (non-blocking): The request mode the give-back asks for — NOW against AFTER_BACKOFF — has no killable mutant in the class.

LDAPReplicationDomain.java:2755, ParkedChangeGiveBackTest.java:143-150, :178-187

Both cases wait for the restart's report inside CHECKPOINTER_BOUND_IN_MS (5 s) and for the connection inside RESTART_BOUND_IN_MS (10 s). With the term mutated to AFTER_BACKOFF, case 2's checkpointer takes the backoff road: restartSession(true), a 2 s wait (consecutive is already 1 after the barrier's restart), the injected failure, the report at ~3 s — inside both bounds; case 1 likewise. The mode is observable without timing: a NOW restart runs restartSession(false) and leaves getConsecutiveSessionRestarts() alone, an AFTER_BACKOFF one moves it.

awaitConnected(RESTART_BOUND_IN_MS,
    "the session was not brought back after the restart which failed");
assertEquals(domain.getConsecutiveSessionRestarts(), 2,
    "the restart the give-back asked for must be run at once, not after the backoff");

Take the number from a run at this head (2 by reading: the barrier's restart, then the retry after the injected failure), then check the mutant moves it to 3 — the same pin killed this mutant on #988.

…wound parked as dependencies A change which waits for another one is parked and stays owned by the replay thread which parked it: getNextUpdate() is what hands it out again, to whichever thread clears the change it was waiting for. A replay which is unwound leaves that thread without the road back - it takes the next delivery off the shared queue - so the change stayed owned by a thread which never came back to it, while every redelivery of a change a replay thread owns is refused as a duplicate. On a domain which then went quiet that change was where this replica's ServerState, and every change behind it from every master, stopped. RemotePendingChanges gives back the changes the calling thread parked and unparks them in the same step, under both locks, so that only one road can hand a change out: a change released while it is still listed as waiting would be handed to the thread getNextUpdate() gives it to and to the thread which takes over the delivery which follows. The changes another thread parked are left alone, as everywhere else. The list of what it released is the one allocation of the method past the locks, sized for every change which is waiting before anything is taken out of the set - the rule getNextUpdate() states for itself, on a road out of a JVM which has just refused an allocation - and the second lock is taken inside the try of the first, since taking a lock another thread holds allocates too. replay() gives them back before the road of the change it was replaying runs, since that road restarts the session and a change which is still owned when the replication server sends it again is turned down. They are handed back without a failure counted against them - they were never applied here - and the session is restarted for them, without the backoff on the roads the owned road skips it on: a thread which is stopping, and one an OutOfMemoryError is ending. On a domain whose session has an owner - the domain itself, going away, or a total update into it from the moment it is asked for - they are released and nothing more, the way abandonReplay() hands a change back there: no session of this thread's is left to restart, and the restart it would ask for is refused where it runs. Which change this thread owns is read before they are given back, and not after. The read is a plain map lookup which allocates nothing, and the give-back below it allocates - it builds the list of what it released and the line which reports each one. A throw from it on the road it exists for would otherwise reach the last resort of replay() with nothing read, and the change this thread was replaying would be left listed, uncommitted and owned by a thread which is ending: the wedge OpenIdentityPlatform#922 is about, one road over. That last resort runs the restart outside the guard on the change this thread owned: a give-back which released the parked changes, asked for the restart and then threw reporting them, or a restart the parked road ran and which threw, both leave a request standing on a thread which may own no change of its own. It asks for the restart of the change it owned after releasing it, as every road which releases a change asks, and runs nothing on a domain whose session has an owner: the request stands for the state checkpointer rather than being spent on a restart which is refused. SessionRestartRequests links the call sites of its lambda and of its atomic reference once, at construction: their first execution allocates, and every request is made on a road where an allocation may be what has just failed. The end-to-end test unwinds the replay of a change which is applied and whose ack runs out of memory: the one road which leaves replay() with no change of this thread's to ask for again, so the restart the give-back asks for is the only one and the test is red without it. The change it parks travels the replication server, and nothing but the restarted session brings it back. The road with an owner is pinned where a test can hold it open, a total update whose request is on its way: the changes are released, and nothing is asked for, counted or reported for them. Who runs the restart is pinned with the replay on the thread of the test and the restart asked to fail once: the thread which gave the changes back meets the failure itself, a stopping thread leaves it to the state checkpointer to meet and report. The stopping thread is stopped by the ack of the change it applied, not before the replay: stopped before it, the change is abandoned unapplied at the top of its first attempt and the abandon road asks for a restart of its own next to the give-back's, so the give-back's request would be pinned by nothing - the case asserts that the change was applied, and a third case runs that other road, which pins the abandon arm of the catch. The count of the restarts in a row pins that the give-back asks for the restart without the backoff: only the restart run again after the failure moves it.

vharseko · 2026-09-17T15:03:35Z

The round on 2d97f6c01c, taken on f8d55997bb - the branch is rebased onto master at d07bb31897 on the way (#982, #987 and #996 landed; the commit went over clean, and its tree is the one #988's restack already sits on).

The road the stopping case ran is asserted

As you say: nothing in the case said which road it ran, and every assertion held on the abandon road too. It reads replayed-updates-ok before the replay and asserts it moved by one after - the count moves on the applied road alone, and no session restart zeroes it, so the reading is not a window. Measured with the case in its previous shape, the flag up before the replay: red 0/1 on that line, the change must be applied and its replay unwound by the ack, not abandoned unapplied expected [1] but found [0], every other assertion of the case green.

The abandon arm of the catch has its case

aStoppingThreadHandsBackTheChangeItDidNotApply: the flag up before the replay, so the change is abandoned unapplied at the top of its first attempt and the ack which says so is what runs out of memory - the arm of the catch where the change is still this thread's. It asserts that replayed-updates-ok did not move, that NOTE_REPLAY_ABANDONED_CHANGE was logged for the change - matched on its CSN, since the test error log keeps a record per publisher - and that the parked change was given back. Red with abandonReplay(owned) deleted from the catch: 0/1, a stopping thread must hand back the change it did not apply - Expecting actual not to be empty. The helper takes when the thread is stopped - never, by the ack, before the replay - in place of the boolean.

The mode is pinned

getConsecutiveSessionRestarts() read before the replay is unwound and once the session is back, +1 in both cases: the restart the give-back asks for is run at once and moves nothing, the one run again after the injected failure is the one which waits its backoff out. Relative rather than the absolute 2, which is the same reading with the barrier's restart named. With the give-back asking AFTER_BACKOFF on every road: red 0/2, expected [2] but found [3] in both cases - the same pin as #988's.

Runs

On this head, one JVM per class: ParkedChangeGiveBackTest 3/3, ReplayDuringImportTest 6/6, SessionRestartBackoffTest 3/3, UpdateOperationTest 40/40 - the cases master gained since among them; 52 tests, Skipped: 0. The mutants are the three above; the ones of the previous rounds are not re-measured on this head.

The description is updated to match.

maximthomas

praise: The delta takes both round-5 deferrals and turns each claim of the description into a measured pin.

aStoppingThreadHandsBackTheChangeItDidNotApply (opendj-server-legacy/src/test/java/org/opends/server/replication/plugin/ParkedChangeGiveBackTest.java:218-230) is the first case to reach the abandon arm of replay()'s last resort: abandonReplay(owned) deleted at opendj-server-legacy/src/main/java/org/opends/server/replication/plugin/LDAPReplicationDomain.java:2802 turns its NOTE 309 assertion (:229) red, the other two cases stay green — 1/3 red, as the description says.
assertEquals(domain.getConsecutiveSessionRestarts(), restartsBefore + 1, ...) (ParkedChangeGiveBackTest.java:157, :204) pins the request mode the give-back asks for: giveBackParkedChanges(SessionRestart.AFTER_BACKOFF) on every road (LDAPReplicationDomain.java:2789-2791) reads expected [2] but found [3] in both restart cases — 2/3 red, case 3 green.
sessionRestarts.request(restart) deleted inside giveBackParkedChanges() (LDAPReplicationDomain.java:4208) is red 2/3: case 1 at :152 (the thread never met the injected failure) and case 2 at :197 (the checkpointer had nothing to run) — the one pin the round-5 delta existed for, now measured on this head.
3/3 green at f8d5599, on the Linux CI cell and locally; the production files are identical to the round-5 head, so the round-5 Approve stands on this one.

vharseko · 2026-09-18T16:23:38Z

Closing as superseded: #988 was stacked on this branch's head (f8d5599) and squash-merged into master as 80481f7 14 minutes before this PR's last approval, carrying the whole [#954] change with it (message 318 was reworded there, on purpose). Rebasing onto master now leaves this branch's commit empty - git diff origin/master is clean. Closing #954 by hand against 80481f7.

vharseko requested a review from maximthomas September 9, 2026 06:58

vharseko added bug replication tests Test suites: fixing, enabling, un-disabling concurrency Thread-safety / race-condition bugs labels Sep 9, 2026

This was referenced Sep 9, 2026

[#986] Give back the changes a replay thread the pool stopped had parked #988

Merged

[#922] Give a change back when the replay which owns it is unwound #958

Merged

vharseko force-pushed the issues/954-parked-change-give-back branch from 1b3af39 to 240fadc Compare September 9, 2026 10:13

vharseko force-pushed the issues/954-parked-change-give-back branch from 240fadc to 13be038 Compare September 11, 2026 04:56

vharseko requested review from maximthomas and removed request for maximthomas September 11, 2026 04:59

vharseko mentioned this pull request Sep 11, 2026

[#1014] Report a ReplicaOfflineMsg as forwarded only to a peer which can decode it #1019

Open

vharseko force-pushed the issues/954-parked-change-give-back branch from 13be038 to 48420fa Compare September 11, 2026 18:35

vharseko force-pushed the issues/954-parked-change-give-back branch from 48420fa to 06ace4f Compare September 12, 2026 13:19

vharseko mentioned this pull request Sep 15, 2026

[#1039] Report a total update whose session stops before the DoneMsg as a failed import #1044

Open

maximthomas requested changes Sep 16, 2026

View reviewed changes

vharseko force-pushed the issues/954-parked-change-give-back branch from 06ace4f to fda30af Compare September 16, 2026 07:26

vharseko requested a review from maximthomas September 16, 2026 07:26

vharseko added the java Changes to Java sources label Sep 16, 2026

maximthomas approved these changes Sep 16, 2026

View reviewed changes

vharseko force-pushed the issues/954-parked-change-give-back branch from fda30af to 42284c5 Compare September 16, 2026 11:02

vharseko requested a review from maximthomas September 16, 2026 11:10

maximthomas requested changes Sep 16, 2026

View reviewed changes

vharseko force-pushed the issues/954-parked-change-give-back branch 2 times, most recently from c1403da to 939fd02 Compare September 16, 2026 14:24

vharseko requested a review from maximthomas September 16, 2026 14:25

vharseko mentioned this pull request Sep 17, 2026

[#1036] Assert that a parked change is handed out, not which thread hands it out #1037

Open

maximthomas approved these changes Sep 17, 2026

View reviewed changes

vharseko force-pushed the issues/954-parked-change-give-back branch from 939fd02 to 2d97f6c Compare September 17, 2026 12:11

vharseko mentioned this pull request Sep 17, 2026

Replication: a change released while a total update which never begins owns the session is not asked for again until the next failed replay #1061

Open

vharseko requested a review from maximthomas September 17, 2026 12:59

maximthomas approved these changes Sep 17, 2026

View reviewed changes

vharseko force-pushed the issues/954-parked-change-give-back branch from 2d97f6c to f8d5599 Compare September 17, 2026 15:03

vharseko requested a review from maximthomas September 17, 2026 15:03

vharseko mentioned this pull request Sep 17, 2026

[#1061] Ask for the session restart under an owner as well, and leave it to the state checkpointer #1062

Open

maximthomas approved these changes Sep 18, 2026

View reviewed changes

vharseko closed this Sep 18, 2026

vharseko mentioned this pull request Sep 18, 2026

Replication: a change parked as a dependency is left owned by a thread which never comes back to it #954

Closed

Uh oh!

Conversation

vharseko commented Sep 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

The give-back unparks

Where it runs

Which throws reach it

Tests

Built on #958

Left out

Uh oh!

vharseko commented Sep 9, 2026

The ordinal is 318

The end-to-end test unwinds the replay another way

The last resort runs the restart for these too

Two javadocs which the third commit made untrue

Runs

Uh oh!

vharseko commented Sep 11, 2026

The owner is read before the parked changes are given back

Two comments the fourth commit made untrue

The end-to-end test takes the configured budget

The ordinals

Runs

Uh oh!

vharseko commented Sep 11, 2026

The javadoc of the ownership index

One comment the fifth commit made untrue

The barrier of the end-to-end test

Ordinals

Runs

Uh oh!

vharseko commented Sep 12, 2026

Uh oh!

maximthomas left a comment

Choose a reason for hiding this comment

Uh oh!

vharseko commented Sep 16, 2026

The allocation comes first, the request comes first

The end-to-end test pins the restart the give-back asks for

The count and the line

The abandon road

Text

Runs

Uh oh!

maximthomas left a comment

Choose a reason for hiding this comment

Uh oh!

vharseko commented Sep 16, 2026

The give-back asks the way the owned road asks

A thread which is stopping leaves the restart standing

The last resort

Text

Runs

Uh oh!

maximthomas left a comment

Choose a reason for hiding this comment

PR #985 — review feedback, round 3 (head 42284c5)

Uh oh!

vharseko commented Sep 16, 2026

The request which can throw on its first execution

The give-back's owner road, and the last resort's

The parked road's run, and the stopping thread's

The no-backoff term on the OutOfMemoryError road

The second lock

The three comments

The test's way out

Runs

Uh oh!

maximthomas left a comment

Choose a reason for hiding this comment

Uh oh!

vharseko commented Sep 17, 2026

The stopping thread is stopped by the ack

The give-back under an owner asks for nothing: chosen, and the hold-back is master's

The two gates on the run

Runs

Uh oh!

maximthomas left a comment

Choose a reason for hiding this comment

vharseko commented Sep 9, 2026 •

edited

Loading

PR #985 — review feedback, round 3 (head `42284c5`)